replace incomplete compound types #3643
Conversation
kroening
requested review from
allredj,
chrisr-diffblue,
martin-cs,
peterschrammel,
pkesseli,
romainbrenguier and
smowton
as code owners
kroening
force-pushed
the
incomplete_compound_types
branch
from
1478982 to
7da25a1
Compare
There was a problem hiding this comment.
These are large changes without a commit message (plus the commit subjects don't parse for me), cpplint requests one assert to be replaced, and a rebase is required.
For the record: I am no longer going to review PRs containing commits without proper commit messages. Unfortunately GitHub does not support providing feedback on commit messages. The pull-request template contains a link to guidance on how to write commit messages. Should I receive a review request for a PR containing a commit violating commit-message guidance I'll reassign it to the author without comment. I am simply not willing to spend my time reverse engineering what a commit is about and why it is being proposed or why a change is the right thing to do.
kroening
force-pushed
the
incomplete_compound_types
branch
from
4087771 to
ede4512
Compare
There was a problem hiding this comment.
✔️
Passed Diffblue compatibility checks (cbmc commit: ede4512).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/95950633
| @@ -418,13 +425,6 @@ void c_typecheck_baset::typecheck_redefinition_non_type( | |||
| PRECONDITION(old_symbol.type.id() != ID_symbol_type); | |||
| old_symbol.type=new_symbol.type; | |||
| } | |||
| else if((final_old.id()==ID_incomplete_c_enum || | |||
There was a problem hiding this comment.
Shouldn't we check the new is-incomplete flag here?
src/ansi-c/c_typecheck_type.cpp
Outdated
| else | ||
| UNREACHABLE; | ||
| // mark as incomplete | ||
| compound_symbol.type.set(ID_incomplete, true); |
There was a problem hiding this comment.
Provide a setter for this on struct_union_typet rather than directly twiddle IDs?
src/cpp/cpp_instantiate_template.cpp
Outdated
| // "template_class_instance", to be elaborated later. | ||
| symbolt new_symbol; | ||
| new_symbol.name=identifier; | ||
| new_symbol.pretty_name=template_symbol.pretty_name; | ||
| new_symbol.location=template_symbol.location; | ||
| new_symbol.type=typet(ID_incomplete_struct); | ||
| new_symbol.type = typet(ID_struct); |
There was a problem hiding this comment.
Use constructor? Or add a static struct_typet::make_incomplete_struct(const irep_idt &tag) or something?
| new_type.set(ID_tag, new_symbol->base_name); | ||
| new_type.set(ID_incomplete, true); |
There was a problem hiding this comment.
Use struct_union_typet interface?
kroening
force-pushed
the
incomplete_compound_types
branch
from
ede4512 to
0df2c36
Compare
There was a problem hiding this comment.
✔️
Passed Diffblue compatibility checks (cbmc commit: 0df2c36).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/96342190
…c_enum_type 1) The incomplete_struct type is replaced by the struct type with ID_incomplete set. 2) The incomplete_union type is replaced by the union type with ID_incomplete set. 3) The incomplete_c_enum type is replaced by the c_enum type with ID_incomplete set. The question whether any of the compound types is incomplete is only relevant to the front-ends; outside of the front-end, the existence of three further types only adds unnecessary complexity. This change further establishes the invariant that a struct_tag, union_tag, c_enum_tag always points to a struct, union, c_enum type, respectively.
…compound type A struct_tag, union_tag, c_enum_tag always points to a struct, union, c_enum type, respectively. This is now reflected in the return type of the method, which implies that a number of explicit casts and checks can be removed.
tautschnig
force-pushed
the
incomplete_compound_types
branch
from
0df2c36 to
4e031ff
Compare
There was a problem hiding this comment.
@kroening Thank you very much for the elaborate commit messages, these are very helpful!
tautschnig
assigned peterschrammel, martin-cs and chrisr-diffblue and unassigned kroening
There was a problem hiding this comment.
✔️
Passed Diffblue compatibility checks (cbmc commit: 4e031ff).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/96604942
A key benefit will be that follow_tag will then be able to return the corresponding compound type.